perf(tests): reduce runtime and GPU memory - #461
Conversation
Greptile SummaryThe PR reorganizes CI tests by simulation and GPU requirements to reduce runtime and peak GPU memory.
Confidence Score: 5/5The PR appears safe to merge because no eligible blocking failure remains. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| .github/workflows/main.yml | Splits the test suite into documentation, parallel non-simulation, serial simulation, isolated distributed-GPU, and remaining GPU lanes. |
| tests/conftest.py | Adds deferred task discovery and source-based simulation classification used by the new CI lanes. |
| embodichain/lab/sim/sim_manager.py | Makes cleanup flushing return immediately when no destruction task was queued while retaining collection and scene synchronization after actual cleanup. |
| embodichain/lab/sim/objects/articulation.py | Converts detached drive tensors to scalar values or NumPy arrays before forwarding them to the simulation entity. |
| embodichain/lab/sim/solvers/srs_solver.py | Accepts Torch target-pose tensors in the CPU solver and converts them safely to NumPy. |
| pyproject.toml | Registers resource-oriented pytest markers, warning filters, and the default exclusion of slow coverage. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Collected tests] --> B{Documentation?}
B -->|Yes| C[Documentation lane]
B -->|No| D{GPU marked?}
D -->|No| E{Requires simulation?}
E -->|No| F[Four-worker pure-Python lane]
E -->|Yes| G[Serial real-simulation lane]
D -->|Yes| H{Distributed training?}
H -->|Yes| I[Isolated distributed-GPU lane]
H -->|No| J[Serial remaining-GPU lane]
Reviews (2): Last reviewed commit: "fix(ci): keep CUDA visible during test c..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
This PR restructures test execution to reduce overall wall time and GPU memory pressure by marking and routing tests based on simulation/GPU/task-discovery needs, and by tightening a few hot-path teardown/initialization behaviors.
Changes:
- Adds/adjusts pytest markers (
requires_sim,no_sim,requires_tasks,gpu,slow,xdist_group) and updates collection logic to better separate pure-Python vs real-simulation vs GPU workloads. - Improves simulation cleanup behavior by making
SimulationManager.flush_cleanup_queue()return immediately when there’s nothing pending, avoiding unnecessary global waits. - Updates CI workflow to run tests in staged lanes (docs, pure-Python xdist, real-sim, isolated distributed GPU, remaining GPU).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/toolkits/test_batch_convex_collision.py | Marks CUDA-dependent collision test as gpu and reorders imports. |
| tests/sim/test_sim_profiler.py | Marks profiler unit tests as no_sim to keep them out of the simulation lane. |
| tests/sim/test_sim_manager.py | Adds regression coverage for cleanup-queue behavior and marks mock-only tests as no_sim. |
| tests/sim/planners/test_toppra_batched.py | Reduces default env/batch sizes and marks heavier multiprocessing coverage as slow. |
| tests/sim/planners/test_curobo_planner.py | Adds a module fixture to restore Torch TF32/matmul precision after cuRobo tests. |
| tests/sim/objects/test_rigid_constraint.py | Marks mock-only constraint config tests as no_sim. |
| tests/learning/test_shared_rollout.py | Splits mock-only vs real-sim + tasks tests using no_sim / requires_* markers. |
| tests/learning/test_rl.py | Marks RL training tests as requires_sim, requires_tasks, and slow. |
| tests/learning/test_rl_distributed.py | Marks distributed torchrun test as gpu and slow. |
| tests/learning/test_newton_planar_reach.py | Marks expensive APG training test as slow. |
| tests/gym/envs/test_replay.py | Marks replay env tests as requires_sim and slow. |
| tests/gym/envs/test_profiler_integration.py | Marks integration test as requires_sim. |
| tests/gym/envs/test_embodied_env.py | Marks env tests as requires_sim. |
| tests/gym/envs/test_base_env.py | Marks env tests as requires_sim. |
| tests/data_pipeline/test_online_data.py | Avoids starting the sim subprocess for sampling-only tests to reduce teardown waits. |
| tests/conftest.py | Defers task discovery to requires_tasks tests; adds real-sim classification and xdist_group routing; defers DexSim init. |
| pyproject.toml | Sets default -m "not slow" and registers new markers, plus warning filtering. |
| embodichain/lab/sim/solvers/srs_solver.py | Fixes __all__, adds future annotations, and supports Torch tensors for target poses. |
| embodichain/lab/sim/sim_manager.py | Makes cleanup flush skip global waits when nothing was drained. |
| embodichain/lab/sim/objects/articulation.py | Normalizes drive args to Python scalars when appropriate to reduce tensor->numpy friction. |
| .github/workflows/main.yml | Splits CI tests into docs/pure/real-sim/distributed-GPU/remaining-GPU stages. |
Suppressed comments (2)
.github/workflows/main.yml:168
- Same mark-expression issue as above:
-m "requires_sim and not gpu"overrides the default-m "not slow", soslowreal-simulation tests will run in this job.
echo "Real-simulation tests (one process, GPU tests excluded)"
export HF_ENDPOINT=https://hf-mirror.com
pytest tests --ignore=tests/docs -m "requires_sim and not gpu"
.github/workflows/main.yml:181
- This GPU lane also overrides the default
-m "not slow"by passing-m gpu, so anygpu and slowtests will be included here (even though slow tests are intended to be excluded by default). If the isolated distributed job is the intended home for slow GPU coverage, the remaining GPU suite should explicitly excludeslow.
- name: Run remaining GPU tests serially
run: |
echo "Dedicated GPU test suite"
export HF_ENDPOINT=https://hf-mirror.com
pytest tests --ignore=tests/docs \
--ignore=tests/learning/test_rl_distributed.py --run-gpu -m gpu
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "Pure-Python tests (GPU hidden, four workers)" | ||
| CUDA_VISIBLE_DEVICES="" pytest tests --ignore=tests/docs \ | ||
| -m "not requires_sim and not gpu" -n 4 --dist loadgroup |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Suppressed comments (3)
.github/workflows/main.yml:168
- Same as the xdist lane: supplying
-mhere overrides the repo-wide-m "not slow"addopts. If the goal is to keep slow tests out of the default CI run, addnot slowto this expression as well.
echo "Real-simulation tests (one process, GPU tests excluded)"
export HF_ENDPOINT=https://hf-mirror.com
pytest tests --ignore=tests/docs -m "requires_sim and not gpu"
.github/workflows/main.yml:153
- This workflow no longer runs
tests/gen_sim/scene_engine(it only imports the pipeline). That removes CI coverage for Scene Engine behavior regressions; consider keeping the targeted tests in CI (even if they remain outside the staged simulation/GPU lanes).
- name: Verify Scene Engine gensim installation
run: |
python -c "import matplotlib, numpy, open3d, requests, scipy, shapely, trimesh; from PIL import Image; import embodichain.gen_sim.scene_engine.pipeline.generate"
.github/workflows/main.yml:162
pyproject.tomlsetsaddopts = ["-m", "not slow"], but passing-mhere overrides that default. As a result,@pytest.mark.slowtests can be (re)selected in the non-simulation xdist lane, increasing CI wall time and GPU memory pressure contrary to this PR's intent. Includenot slowin the lane selector (or avoid overriding-m).
This issue also appears on line 166 of the same file.
pytest tests --ignore=tests/docs \
-m "not requires_sim and not gpu" -n 4 --dist loadgroup
Description
This PR reduces unit-test wall time and GPU memory pressure by separating four-worker non-simulation, serial real-simulation, distributed-GPU, and remaining GPU workloads according to their resource needs.
It also removes redundant simulation cleanup waits, defers task discovery and renderer initialization until required, keeps mock-only tests out of the simulation lane, marks expensive coverage explicitly, and prevents cuRobo's process-wide TF32 setting from contaminating later solver tests. CUDA remains visible in the non-simulation lane because Open3D native imports and Scene Engine tests require it. No dependencies are changed.
Measured impact
GPU memory was sampled every 250 ms from each pytest process tree with
nvidia-smi.For comparable non-GPU coverage, wall time drops by about 9m 18.5s (33.0%). Isolating distributed training lowers the observed aggregate GPU peak from 12,709 MiB to 10,748 MiB (15.4%). The current non-simulation xdist lane excludes explicitly GPU-marked tests while retaining CUDA visibility for Open3D and Scene Engine; its measured peak is 288 MiB.
Type of change
Screenshots
Not applicable.
Validation
black .— 600 files unchangedgit diff --checkpassed (actionlintwas not installed in the local image)Checklist
black .command to format the code base.